Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: add workaround to fix build-temp-types on Netlify #7495

Merged
merged 2 commits into from
Mar 28, 2022
Merged

fix: add workaround to fix build-temp-types on Netlify #7495

merged 2 commits into from
Mar 28, 2022

Conversation

fwouts
Copy link
Contributor

@fwouts fwouts commented Mar 28, 2022 •
edited
Loading

Description

This fixes an issue that occurs in the Netlify build, as reported by @patak-dev in #6856 (comment)

This is caused by postcss-load-config missing postcss as a peer dependency, causing a crash when running tsc since it cannot find the corresponding type definitions because of pnpm's strictness. Thank you @bluwy for the idea!

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@patak-dev patak-dev requested a review from bluwy March 28, 2022 13:37
@fwouts
Copy link
Contributor Author

fwouts commented Mar 28, 2022

I can also send a PR for postcss-load-config but I won't have time for a couple of days at least (FYI @ai).

bluwy
bluwy approved these changes Mar 28, 2022
View reviewed changes
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat workaround!

@ai
Copy link

ai commented Mar 28, 2022

postcss-load-config is using postcss only in types https://github.com/postcss/postcss-load-config/blob/main/src/index.d.ts#L4=

Not in the code.

@patak-dev patak-dev merged commit 01bc415 into vitejs:main Mar 28, 2022
@patak-dev
Copy link
Member

@fwouts thanks for the PR, I can confirm that this fixed the issue in Netlify

@bluwy
Copy link
Member

bluwy commented Mar 28, 2022

@ai Vite bundles the types too so it needs to be a peer dep to be bundled properly. Maybe that's out of scope for the package, but I think it's fair to still add an optional peer dep for it so there's an explicit link instead on relying on hoisted types. It could help postcss-load-config too if there are changes to postcss's types.

@ai
Copy link

ai commented Mar 28, 2022

Optional peer is a good option. Send PR when you will have time.

@bluwy bluwy mentioned this pull request Mar 29, 2022
Merged
2 tasks
@brc-dd brc-dd mentioned this pull request Nov 30, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bluwy bluwy bluwy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fwouts @ai @patak-dev @bluwy